-
Notifications
You must be signed in to change notification settings - Fork 0
Extend origins option to allow a function #12
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
1d6488a to
1c0aa2f
Compare
…array and remove original implementation
1c0aa2f to
0904bb6
Compare
origins to allow more sensible origin verificationorigins option more flexible and similar to later versions of socket.io
Seinzu
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we need to fix the default case before we can go ahead. The rest of it looks good.
|
I'm not a big fan of the way the default setup works but it doesn't seem to be blocking |
8f53204 to
fad9007
Compare
2e1f680 to
322d867
Compare
|
I've updated this by changing the |
Seinzu
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I haven't tested this version (but have tested similar implementations). I think this is the lowest risk thing we can do for now.
origins option more flexible and similar to later versions of socket.ioorigins option to allow a function
origins option to allow a function
This is to support custom origin checking in the Overleaf real-time service.
Changes:
Overhauloriginsoption to allow a regex, function, string or boolean, or an array of any mix of theseChangeoriginsoption default to disallow any originhtmlfiletransportflashsockettransportThis still falls back to checking the referrer header when origin is missing. This is a questionable practice and modern versions of socket.io don't do this. The referrer header also has the path, unlike the origin, which means we have to handle that in the
originsoption, probably using a regex. I've opted for a a minimal change that simply allows a function to be passed into theoriginsoption, which receives the origin/referrer and the request object. If theoriginssetting is not a function, it behaves exactly as before, and the default is still to allow any origin.This will fix https://github.com/overleaf/internal/issues/22644, in combination with some small tweaks to the check the origin in the
real-timeservice.We'll need to create a tag (probably
0.9.19-overleaf-11) once this is merged so that we can point thereal-timedependency to the new version.